Refactors TypeQuery #12035
Refactors TypeQuery #12035
Conversation
| this.type = type; | ||
| } | ||
|
|
||
| public TypeQueryBuilder() { |
There was a problem hiding this comment.
can we make this constructor private? null should not be an allowed value from the outside right?
There was a problem hiding this comment.
yes but you could always do new TypeQueryBuilder((String) null)
|
left a few comments |
|
@javanna Thanks for the review. I updated the PR accordingly. |
| this.type = BytesRefs.toBytesRef(type); | ||
| } | ||
|
|
||
| public TypeQueryBuilder(BytesRef type) { |
There was a problem hiding this comment.
Maybe we could make this constructor package private, given that only our own parser will use it, while the java api exposes the existing one that takes a String as argument?
There was a problem hiding this comment.
Good point, will do.
|
did another round, left some comments |
|
@javanna I updated the PRs and responded to the comments. |
| if (randomTypes.length == 0) { | ||
| return MetaData.ALL; | ||
| } | ||
| return randomFrom(randomTypes); |
There was a problem hiding this comment.
I think the whole method could just be:
return randomBoolean ? MetaData.ALL : randomFrom(currentTypes);
There was a problem hiding this comment.
we can't random pick from an empty array.
There was a problem hiding this comment.
then handle when currentTypes is empty and make it all, but don't call getRandomTypes for nothing. Why would you need to create an array composed of a random number of types out of the current types, to then keep only one. Just keep one from original array? Or am I missing something?
There was a problem hiding this comment.
Oh I see yes I should directly access currentTypes instead.
alexksikes
force-pushed
the
feature/query-refactoring-type-query
branch
from
d6d3818 to
81b54e2
Compare
|
@javanna it's rebased, you can take a look. Thank you. |
|
LGTM |
Relates to elastic#10217 Closes elastic#12035 This PR is against the query-refactoring branch.
alexksikes
force-pushed
the
feature/query-refactoring-type-query
branch
from
81b54e2 to
4d4dc5c
Compare
Relates to elastic#10217 Closes elastic#12035 This PR is against the query-refactoring branch.
clintongormley
added
:Search/Search
Relates to #10217
This PR is against the query-refactoring branch.